Skip to content

Conversation

vLuckyyy
Copy link
Member

No description provided.

Copy link

coderabbitai bot commented Nov 10, 2024

Walkthrough

This pull request updates the project’s build setup by removing the Java 17 build script and adding a new Java 21 configuration. Maven repositories are changed, replacing some URLs with new ones. The API dependency switches from Spigot to Paper with adjusted scopes. The core plugin updates dependencies, plugin metadata, and removes test configurations. The main plugin class now uses an interface and a provider pattern for holograms. Configuration classes rename fields and add migrations. Several new classes support hologram management via different providers. Event classes are refactored to share a common base. Position handling moves from a custom record to using Bukkit’s Location directly. Various simplifications and cleanups are applied across the codebase, including removing unused serializers and test code.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Outside diff range and nitpick comments (2)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java (2)

7-7: Add a quick comment explaining the name format.

A simple comment would help others understand what the format represents.

+    /** Format: heads_worldname_x_y_z */
     private static final String HOLOGRAM_NAME_PREFIX = "heads_%s_%s_%s_%s";

17-19: Consider using Pattern.compile for better performance.

If this gets called often, a pre-compiled pattern would be faster.

+    private static final Pattern INVALID_CHARS = Pattern.compile("[^a-zA-Z0-9_-]");
+
     private static String sanitize(String input) {
-        return input.replaceAll("[^a-zA-Z0-9_-]", "_");
+        return INVALID_CHARS.matcher(input).replaceAll("_");
     }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 7e5e4b1 and 49baa20.

📒 Files selected for processing (8)
  • buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts (1 hunks)
  • buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts (1 hunks)
  • lobbyheads-api/build.gradle.kts (1 hunks)
  • lobbyheads-core/build.gradle.kts (3 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (3 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java (0 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (3 hunks)
💤 Files with no reviewable changes (1)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
✅ Files skipped from review due to trivial changes (1)
  • buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts
🧰 Additional context used
🪛 Gitleaks
lobbyheads-api/build.gradle.kts

8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🔇 Additional comments (20)
lobbyheads-api/build.gradle.kts (1)

8-8: Nice improvement!

Using compileOnlyApi for the Spigot API is perfect since the server already provides it at runtime. This keeps your plugin lightweight.

🧰 Tools
🪛 Gitleaks

8-8: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts (2)

14-14: LGTM! Repository added for UniHologram Library.

The new Maven repository looks good and aligns with the PR objective of switching to UniHologram.


Line range hint 1-15: Double-check dependency resolution.

Let's make sure all dependencies can still be resolved after removing the old repository.

✅ Verification successful

All dependencies look good after repository changes

I've checked all the build files and dependencies. None of them use packages from the removed repository, and all current dependencies are pointing to repositories that are still in the configuration. Everything should continue working as expected.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining references to packages from the old repository
rg -l "pl.minecodes" .

# Look for potential build files that might need updates
fd -e gradle -e gradle.kts

Length of output: 452


Script:

#!/bin/bash
# Let's check the dependencies in the build files to see if any still reference the old repository
rg -l "minecodes" lobbyheads-api/build.gradle.kts lobbyheads-core/build.gradle.kts

# Also check the actual dependency declarations
rg "implementation|api|compile" lobbyheads-api/build.gradle.kts lobbyheads-core/build.gradle.kts

Length of output: 1964

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java (1)

1-5: LGTM! Good choice on package visibility.

The class is properly scoped to its package, which keeps things neat and tidy.

lobbyheads-core/build.gradle.kts (2)

69-70: Check if ProtocolLib is still needed.

The removal of ProtocolLib dependency needs verification since it's still downloaded in the runServer task.

#!/bin/bash
# Search for ProtocolLib usage in the code
rg -l "ProtocolLib|protocollib" --type java

37-39: Looks good! Let's verify the dependency.

The new hologram library is properly configured with the CMILib exclusion.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (3)

26-27: Looking good! Clean import additions.

The new imports match perfectly with the switch to UniHologram.

Also applies to: 35-35


50-51: Nice formatting improvement!

The split makes the code easier to read.


74-77: Let's double-check the hologram setup.

The switch to UniHologram looks good, but let's verify that all hologram-related code has been updated.

✅ Verification successful

All hologram-related code has been properly updated

The codebase shows a complete transition to UniHologram:

  • All hologram classes use the new UniHologram imports
  • No traces of HoloEasy found
  • HologramService and HologramController properly integrated with the new provider
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any remaining HoloEasy references
rg -i "holoeasy" 

# Look for other hologram-related classes that might need updates
rg "Hologram" -g "*.java"

Length of output: 6751

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (11)

3-4: Import added successfully

The static import for generateHologramName looks good.


12-15: Updated imports to UniHologram library

The new imports from UniHologram are correctly added.


30-32: New fields added

Fields headManager and provider are properly added.


33-39: Constructor updated

The constructor now includes headManager and provider, which is great.


43-44: Fields initialized correctly

Fields headManager and provider are initialized properly.


48-51: Code update looks good

The placeholder handling and location conversion are correctly implemented.


53-55: Hologram creation updated successfully

Holograms are now created using the new provider, which is correct.


63-66: Loading holograms updated

The hologram loading loop using headManager looks good.


70-71: Check the hologram type casting

Please ensure that casting to Hologram<Location> is safe to avoid any issues.


80-83: Hologram updates look correct

The holograms are updated properly with the new manager.


90-94: Confirm Java version compatibility

The instanceof pattern matching requires Java 16 or higher. Make sure the project supports this version.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (1)

70-71: Consider adding error handling

The hologram removal could benefit from some basic error logging when the hologram isn't found.

-        this.provider.getHologram(generateHologramName(position))
-            .ifPresent(hologram -> ((Hologram<Location>) hologram).clear());
+        this.provider.getHologram(generateHologramName(position))
+            .ifPresentOrElse(
+                hologram -> ((Hologram<Location>) hologram).clear(),
+                () -> System.out.println("No hologram found at position: " + position)
+            );
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 49baa20 and 594096d.

📒 Files selected for processing (1)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (2 hunks)
🔇 Additional comments (5)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (5)

12-15: Nice job with the new imports!

The switch to UniHologram's classes looks clean and organized.


30-31: Good field organization!

The switch from pool-based to provider-based approach looks cleaner.


33-44: Constructor looks better now!

The removal of direct Plugin dependency and addition of HologramProvider makes the class more focused and easier to test.


90-94: Visibility handling looks good!

The new visibility system using PlayerVisibility interface is a nice improvement.


48-55: Double-check the text processing logic

While the hologram creation looks good, we should verify that the text processing works the same way as before.

Let's check if the text processing matches the old implementation:

✅ Verification successful

Let me check for any previous text processing logic to compare with.


Text processing looks good! 👍

The old and new implementations handle text processing in the same way:

  1. Replace {PLAYER} with player name
  2. Process PlaceholderAPI placeholders
  3. Deserialize with MiniMessage
  4. Serialize with section serializer
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any other text processing logic in the old code
rg -A 3 "PlaceholderAPI.setPlaceholders" 

Length of output: 781


Script:

#!/bin/bash
# Look for git history of the file to see previous implementation
git log -p lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java

Length of output: 13117

@vLuckyyy vLuckyyy changed the title Use UniHologram Library instead of HoloEasy. GH-72 Use UniHologram Library instead of HoloEasy. Nov 13, 2024
@imDMK imDMK removed their request for review December 9, 2024 10:10
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java (1)

57-57: Unused variable
skullData is never used. Remove it if it’s no longer needed.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 594096d and 3a54669.

📒 Files selected for processing (13)
  • buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts (1 hunks)
  • lobbyheads-core/build.gradle.kts (3 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (3 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/SoundSerializer.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProvider.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderNotFoundException.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/fancyholograms/FancyHologramsProvider.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
🔇 Additional comments (33)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java (3)

9-10: Good imports
Using UUID and TimeUnit is a neat addition. No issues here.


60-61: Smooth scheduling
Placing the update call in a scheduled task helps avoid blocking. Nice approach!


64-65: Clear ownership setting
Assigning the skull’s owner with the player’s UUID is straightforward and effective.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (13)

3-3: Neat static import!
It simplifies name generation nicely.


8-9: Good choice of direct imports.
This keeps the code clear and organized.


13-13: Import looks fine.
UUID usage seems straightforward.


26-28: Well-structured fields.
Declaring them as final is a helpful design choice.


29-35: Constructor is nicely simplified.
Easy to understand and maintain.


39-40: Clear assignments.
Keeps references easy to track.


44-44: Consider lightening PAPI dependency.
A flexible bridge might help in optional usage.


45-47: MiniMessage and serialization look great.
It keeps the text formatting flexible.


49-49: Provider usage is tidy.
This new approach is a neat improvement.


55-55: Loop logic is clear.
Neatly iterates over heads.


62-62: Removal step is straightforward.
This is easy to follow.


71-71: Keeps all holograms in sync.
Nice way to ensure updates.


77-77: Offset is a nice touch.
Helps positioning the hologram better.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderNotFoundException.java (1)

1-7: Custom exception looks good.
It clarifies missing provider scenarios.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProvider.java (1)

1-13: Interface-based design is great.
It fosters easy swapping of implementations.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (1)

1-17: Dynamic picking is clever.
Makes it simpler to select a suitable provider at runtime.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/SoundSerializer.java (1)

1-30: Neat serialization approach.
Makes sound configurations easy and consistent.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java (2)

3-3: Everything looks fine.

Nice addition of the new XSound import. No problems spotted here.


33-33: Clear and simple usage of XSound.

This is easier to read and maintain than raw sound calls.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java (2)

3-3: Good import placement.

Neat to see SoundSerializer integrated here.


35-35: Registration of SoundSerializer is clear.

This looks straightforward. Ensure the sounds load as expected.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/fancyholograms/FancyHologramsProvider.java (1)

1-67: Nice provider-based approach.

This new class is laid out clearly and uses FancyHolograms well. Good asynchronous handling for hologram creation and updating. Looks friendly to maintain.

lobbyheads-core/build.gradle.kts (5)

34-34: Dependency for XSeries looks good.

This is the correct version and should help with sound features.


36-36: Compile-only FancyHolograms is well-chosen.

This ensures flexibility without forcing the plugin dependency.


66-66: Depend on PlaceholderAPI only.

Seems like a simpler setup. Everything else is optional now.


67-67: Great use of softDepend.

This broadens compatibility with other plugins nicely.


107-109: Re-enable quality checks and tests if possible.

Similar to a prior comment about turning tests and checkstyle back on. It keeps the project tidy.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (3)

18-19: Looking good! 👍

These imports match perfectly with the switch to the new hologram system.


49-50: Nice formatting! 👍

The split makes the code easier to read.


76-82: Clean and clear service setup! 👍

The new HologramService setup looks great with the hologram provider parameter.

@vLuckyyy vLuckyyy changed the title GH-72 Use UniHologram Library instead of HoloEasy. GH-72 Create universal hologram provider instead of HoloEasy. Jan 11, 2025
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (2)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (1)

72-73: Provider selection needs fallback

As mentioned before, the plugin will crash if no hologram plugin is available. Consider adding a no-op provider as fallback.

lobbyheads-core/build.gradle.kts (1)

84-106: Quality checks still disabled

The checkstyle and test tasks are still commented out, which could let issues slip through.

🧹 Nitpick comments (5)
README.md (2)

9-9: Extra slash in Discord badge URL

There are two consecutive slashes after main/, producing
…/main//chat-with-us-on-discord.svg. Some CDNs tolerate this, but others return 404.

-https://raw.githubusercontent.com/vLuckyyy/badges/main//chat-with-us-on-discord.svg
+https://raw.githubusercontent.com/vLuckyyy/badges/main/chat-with-us-on-discord.svg

60-60: Tone tweak (optional)

“One exclamation mark is plenty 🙌.”
Consider dropping it for a slightly calmer tone:

-We wholeheartedly welcome your contributions to LobbyHeads!
+We wholeheartedly welcome your contributions to LobbyHeads.
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (2)

7-7: Consider making this class final.

Since this is a utility class that shouldn't be extended, making it final would be good practice.

-public class HologramProviderPicker {
+public final class HologramProviderPicker {

9-19: Good provider selection logic, but consider making it static.

The method doesn't use any instance state, so it could be static. Also, the priority order (FancyHolograms before DecentHolograms) works well but could use a comment explaining why.

-    public HologramProvider pickProvider(Plugin plugin) {
+    public static HologramProvider pickProvider(Plugin plugin) {
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java (1)

36-54: Consider adding error handling

The hologram operations could fail if the API has issues. Maybe wrap the DHAPI calls in try-catch blocks to log any problems?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a54669 and 7c01196.

📒 Files selected for processing (47)
  • .editorconfig (1 hunks)
  • .github/CONTRIBUTING.md (1 hunks)
  • .whitesource (0 hunks)
  • LICENSE (1 hunks)
  • README.md (3 hunks)
  • buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts (0 hunks)
  • buildSrc/src/main/kotlin/lobbyheads-java-21.gradle.kts (1 hunks)
  • buildSrc/src/main/kotlin/lobbyheads-java-unit-test.gradle.kts (0 hunks)
  • buildSrc/src/main/kotlin/lobbyheads-repositories.gradle.kts (1 hunks)
  • gradle/wrapper/gradle-wrapper.properties (1 hunks)
  • lobbyheads-api/build.gradle.kts (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApi.java (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApiProvider.java (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java (2 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/HeadManager.java (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadCreateEvent.java (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadEvent.java (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadRemoveEvent.java (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadUpdateEvent.java (1 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/Position.java (0 hunks)
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/PositionAdapter.java (0 hunks)
  • lobbyheads-core/build.gradle.kts (3 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (5 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java (4 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsConfigMigration_2025_07_17.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsPositionMigration_2025_07_17.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/HeadSerializer.java (0 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/PositionTransformer.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadController.java (5 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadManagerImpl.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadRepositoryImpl.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java (3 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockService.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/command/HeadCommand.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramController.java (2 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/particle/ParticleController.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/notification/NotificationAnnouncer.java (1 hunks)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/updater/UpdaterNotificationController.java (1 hunks)
  • lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionAdapterTest.java (0 hunks)
  • lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionTest.java (0 hunks)
  • renovate.json (1 hunks)
💤 Files with no reviewable changes (8)
  • .whitesource
  • buildSrc/src/main/kotlin/lobbyheads-java-17.gradle.kts
  • buildSrc/src/main/kotlin/lobbyheads-java-unit-test.gradle.kts
  • lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionAdapterTest.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/HeadSerializer.java
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/PositionAdapter.java
  • lobbyheads-core/test/com/eternalcode/lobbyheads/position/PositionTest.java
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/position/Position.java
✅ Files skipped from review due to trivial changes (8)
  • .github/CONTRIBUTING.md
  • gradle/wrapper/gradle-wrapper.properties
  • LICENSE
  • buildSrc/src/main/kotlin/lobbyheads-java-21.gradle.kts
  • renovate.json
  • lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadEvent.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/notification/NotificationAnnouncer.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsConfigMigration_2025_07_17.java
🚧 Files skipped from review as they are similar to previous changes (6)
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/ConfigurationService.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/sound/SoundController.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramNameUtil.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockController.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/implementation/HeadsConfiguration.java
  • lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramService.java
🧰 Additional context used
🧬 Code Graph Analysis (3)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramController.java (3)
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadCreateEvent.java (1)
  • HeadCreateEvent (11-27)
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadRemoveEvent.java (1)
  • HeadRemoveEvent (14-30)
lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadUpdateEvent.java (1)
  • HeadUpdateEvent (11-27)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (2)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java (1)
  • DecentHologramsProvider (13-55)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/fancyholograms/FancyHologramsProvider.java (1)
  • FancyHologramsProvider (15-67)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (1)
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java (1)
  • HologramProviderPicker (7-20)
🪛 LanguageTool
README.md

[style] ~60-~60: Using many exclamation marks might seem excessive (in this case: 5 exclamation marks for a text that’s 3038 characters long)
Context: ...welcome your contributions to LobbyHeads! Please refer to our [contribution guide...

(EN_EXCESSIVE_EXCLAMATION)

🔇 Additional comments (33)
README.md (1)

25-30: Confirm GIF branch path

The GIFs now reference the master branch. If the default branch is main, these links will break.

-https://github.com/EternalCodeTeam/LobbyHeads/blob/master/
+https://github.com/EternalCodeTeam/LobbyHeads/blob/main/

Please double-check that the images load after the merge.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/updater/UpdaterNotificationController.java (1)

31-31: Configuration field ‘checkUpdates’ verified

I checked and found public boolean checkUpdates = true; in HeadsConfiguration.java, so the rename is correct and everything aligns. No further changes needed—approving this update.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/serializer/PositionTransformer.java (1)

3-3: Position abstraction update looks good.

The switch to commons.bukkit.position.Position and the class rename to PositionTransformer aligns with the broader refactoring effort to use standardized position handling.

Also applies to: 9-9

lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApi.java (1)

4-4: Excellent API improvements!

Adding Javadoc documentation and @NotNull annotations makes the API much clearer for developers. These are great best practices for public interfaces.

Also applies to: 6-8, 11-11

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/particle/ParticleController.java (1)

23-23: Approved: Method and config renames are in place
Everything looks great – getPlayerUuid and the headSettings section are used consistently across the codebase. No further changes needed.

lobbyheads-api/build.gradle.kts (2)

2-2: Java 21 upgrade looks good!

This modernizes the build to use Java 21, which is a good step forward.


8-8: Paper API migration is a solid improvement.

Switching from Spigot to Paper API provides better features and performance. The compileOnlyApi scope is correct here - it prevents Paper from being transitively exposed to API consumers while still allowing compilation.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadRepositoryImpl.java (2)

8-8: Great thread safety improvement!

Converting to CopyOnWriteArrayList prevents concurrent modification issues. Smart defensive programming here.

Also applies to: 19-21


26-29: Excellent atomic operation design.

Moving both list mutations and config saves into the same async blocks ensures consistency. The updateHead method now handles both update and add cases cleanly within the async context.

Also applies to: 34-37, 42-53

lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/HeadManager.java (2)

6-7: Excellent API documentation and nullability contracts.

The @NotNull and @Nullable annotations make the API much safer to use. Clear documentation helps developers understand expected behavior.

Also applies to: 17-17, 22-22, 27-27, 32-32, 37-37


9-11: Well-written Javadoc documentation.

The interface and method documentation is clear and concise. Developers will easily understand how to use this API.

Also applies to: 14-16, 19-21, 24-26, 29-31, 34-36

lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadCreateEvent.java (2)

11-11: Good inheritance refactoring.

Extending HeadEvent reduces code duplication and creates a cleaner event hierarchy. The constructor delegation is properly implemented.

Also applies to: 15-16


19-19: Proper method overrides.

The @Override annotations are correctly placed and the method signatures match the parent class.

Also applies to: 24-24

lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadRemoveEvent.java (3)

9-13: Excellent documentation clarification.

The note about UUID referring to the head owner (not remover) prevents common confusion. Very helpful for API users.


14-14: Consistent inheritance pattern.

Extending HeadEvent maintains consistency with other event classes and reduces duplication. The constructor delegation follows the established pattern.

Also applies to: 18-19


22-22: Proper method overrides.

The @Override annotations are correctly applied and method signatures are consistent.

Also applies to: 27-27

.editorconfig (1)

1-31: Great editor configuration improvements!

The new Java-specific settings will help maintain consistent code style across the project. The IntelliJ IDEA settings look comprehensive and follow good practices, especially the import organization and final parameter generation.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/HologramController.java (2)

29-34: Nice method name improvement!

Renaming createHologram to onHeadCreate makes it much clearer that this is an event handler. The variable renaming from player to playerUuid and offlinePlayer to player also improves readability.


33-33: Good configuration path update.

The change from config.headSection.defaultHeadFormat to config.headSettings.defaultHeadFormat aligns with the configuration restructuring mentioned in the PR summary.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadController.java (3)

23-35: Excellent interface-based design improvement!

Switching from HeadManagerImpl to the HeadManager interface follows good dependency inversion principles and makes the code more testable and flexible.


52-78: Good simplification by removing PositionAdapter calls.

Passing location directly instead of converting it with PositionAdapter.convert() simplifies the code nicely. The updated notification message keys also align well with the configuration restructuring.


73-73: Nice fix for the method name casing.

Correcting getPlayerUUID() to getPlayerUuid() follows proper Java naming conventions.

lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/LobbyHeadsApiProvider.java (3)

5-14: Excellent utility class design!

Making the class final with a private constructor that throws an exception properly prevents instantiation. The Javadoc is also a nice touch for API clarity.


20-26: Great thread safety improvement!

The volatile field with local variable pattern is a good approach to prevent race conditions. The null check with clear exception message makes debugging easier.


31-36: Proper initialization safety checks.

The null check prevents double initialization, which is important for maintaining API consistency. Good defensive programming.

lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/event/HeadUpdateEvent.java (1)

8-27: Nice refactoring!

The change to extend HeadEvent makes the code cleaner by sharing common fields. Good use of @NotNull annotations too.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/block/BlockService.java (1)

5-25: Good move to use the interface!

Using HeadManager instead of the implementation class makes the code more flexible. The simplified method signatures look cleaner too.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/decentholograms/DecentHologramsProvider.java (1)

13-34: Clean implementation of the provider pattern!

Good job scheduling operations on the main thread for safety. The immediate visibility to all players makes sense for lobby heads.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/configuration/migration/HeadsPositionMigration_2025_07_17.java (1)

12-120: Solid migration implementation!

Great job handling all the edge cases - from parsing failures to already-migrated data. The regex pattern looks spot on for the old format.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/HeadsPlugin.java (1)

41-81: Great improvements to the plugin structure!

Love the switch to interfaces and the cleaner dependency injection. The provider pattern for holograms is much more flexible now.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/command/HeadCommand.java (1)

7-89: Nice cleanup!

The refactoring looks good - simpler naming with announcer, better null checks for target blocks, and using the newer getTargetBlockExact API. The permission check extraction to a private method also makes the code cleaner.

lobbyheads-core/build.gradle.kts (1)

64-64: Check soft dependencies setup

Make sure the soft dependencies for DecentHolograms and FancyHolograms are handled properly in your hologram provider picker logic.

lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/HeadManagerImpl.java (1)

3-89: Clean refactoring!

Good job simplifying the API to use Location everywhere and keeping the internal Position conversion hidden. The unmodifiable collection return is also a nice touch for safety.

return new DecentHologramsProvider(plugin);
}

throw new HologramProviderNotFoundException("No supported hologram plugin found.");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing import for HologramProviderNotFoundException.

The code references HologramProviderNotFoundException but doesn't import it. This will cause a compilation error.

Add the missing import at the top of the file.

🤖 Prompt for AI Agents
In
lobbyheads-core/src/main/java/com/eternalcode/lobbyheads/head/hologram/provider/HologramProviderPicker.java
at line 18, the class HologramProviderNotFoundException is used but not
imported, causing a compilation error. Add the appropriate import statement for
HologramProviderNotFoundException at the top of the file with the other imports
to resolve this issue.

Comment on lines +18 to +22
public Head() {
this.world = null;
this.x = this.y = this.z = 0;
this.yaw = this.pitch = 0;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Watch out for null world!

The no-arg constructor sets world to null, which will cause problems when getLocation() is called later.

🤖 Prompt for AI Agents
In lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java around
lines 18 to 22, the no-argument constructor sets the world field to null, which
will cause issues when getLocation() is called. To fix this, initialize the
world field with a valid default World instance or modify the constructor to
require a non-null world parameter, ensuring that world is never null when used
later.

Comment on lines +64 to 66
return this.world.equals(other.world) &&
this.x == other.x && this.y == other.y && this.z == other.z;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Missing null check in equals

The equals method doesn't check if world is null before calling .equals().

Add a null check:

-return this.world.equals(other.world) &&
+return Objects.equals(this.world, other.world) &&
     this.x == other.x && this.y == other.y && this.z == other.z;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return this.world.equals(other.world) &&
this.x == other.x && this.y == other.y && this.z == other.z;
}
return Objects.equals(this.world, other.world) &&
this.x == other.x && this.y == other.y && this.z == other.z;
}
🤖 Prompt for AI Agents
In lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java around
lines 64 to 66, the equals method calls this.world.equals(other.world) without
checking if this.world is null, which can cause a NullPointerException. Fix this
by adding a null check for this.world before calling equals, for example by
using Objects.equals(this.world, other.world) or explicitly checking if
this.world is null and comparing accordingly.

Comment on lines +35 to 41
public @NotNull Location getLocation() {
return new Location(
org.bukkit.Bukkit.getWorld(this.world),
this.x, this.y, this.z,
this.yaw, this.pitch
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add null safety for world

The getLocation() method could throw NPE if world is null or if Bukkit can't find the world.

Consider adding a null check:

 public @NotNull Location getLocation() {
+    if (this.world == null) {
+        throw new IllegalStateException("World is not set");
+    }
+    var world = org.bukkit.Bukkit.getWorld(this.world);
+    if (world == null) {
+        throw new IllegalStateException("World '" + this.world + "' not found");
+    }
     return new Location(
-        org.bukkit.Bukkit.getWorld(this.world),
+        world,
         this.x, this.y, this.z,
         this.yaw, this.pitch
     );
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public @NotNull Location getLocation() {
return new Location(
org.bukkit.Bukkit.getWorld(this.world),
this.x, this.y, this.z,
this.yaw, this.pitch
);
}
public @NotNull Location getLocation() {
if (this.world == null) {
throw new IllegalStateException("World is not set");
}
var world = org.bukkit.Bukkit.getWorld(this.world);
if (world == null) {
throw new IllegalStateException("World '" + this.world + "' not found");
}
return new Location(
world,
this.x, this.y, this.z,
this.yaw, this.pitch
);
}
🤖 Prompt for AI Agents
In lobbyheads-api/src/main/java/com/eternalcode/lobbyheads/head/Head.java around
lines 35 to 41, the getLocation() method may throw a NullPointerException if the
world field is null or if Bukkit.getWorld(this.world) returns null. Add a null
check for the world object returned by Bukkit.getWorld(this.world) before
creating the Location instance. If the world is null, handle it gracefully by
either returning null, throwing a custom exception, or using an Optional to
indicate absence, depending on the intended design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants